Conversation
🗄️ Schema Change: No Changes ✅ |
🛠️ Item Splitting: No Changes ✅ |
|
Size Change: +520 B (+0.1%) Total Size: 497 kB
ℹ️ View Unchanged
|
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (1260652) and published it to npm. You Example: pnpm add @khanacademy/perseus@PR3425If you are working in Khan Academy's frontend, you can run the below command. ./dev/tools/bump_perseus_version.ts -t PR3425If you are working in Khan Academy's webapp, you can run the below command. ./dev/tools/bump_perseus_version.js -t PR3425 |
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
@claude review once |
.../perseus-editor/src/widgets/interactive-graph-editor/start-coords/start-coords-logarithm.tsx
Outdated
Show resolved
Hide resolved
7c67738 to
8976066
Compare
fd16188 to
ada2fad
Compare
8976066 to
680fbf2
Compare
63f729e to
53155ac
Compare
SonicScrewdriver
left a comment
There was a problem hiding this comment.
A couple of small suggestions, mostly about the project at large, but nothing blocking imo.
However, I'm not able to test the editor for this PR's chromatic storybook snapshot. I could have sworn I got Exponential working, but I don't see that available either. It'd be great to be able to test the full widget editors in storybook for additional confidence. Given that we're doing a couple rounds of play testing, I'm still alright to approve anyway.
| @@ -0,0 +1,36 @@ | |||
| /* We have to use !important until wonder blocks is in the shared layer. */ | |||
There was a problem hiding this comment.
I honestly wonder how many of these !importants are necessary. I would love it if we made a task to clean up as many of these as possible as part of our project (aka before LEMS-3686, which might be a while).
There was a problem hiding this comment.
for now it's necessary in editor side. But I agree let's revisit this before our project ends if that ticket linked is something we can pickup as cleanup task.
| const {coords, asymptote} = startCoords; | ||
|
|
||
| // Local state for the asymptote x-value text field so the user can type | ||
| // freely without the field resetting mid-keystroke. Pattern from StartCoordsCircle. |
There was a problem hiding this comment.
The AI really likes to say where it stole things from, but I'm not sure "Pattern from StartCoordsCircle." is particularly helpful
…ema (#3420) ## Summary: PR series to add logarithm graph support to the Interactive Graph widget: 1.▶️ [Add logarithm graph type definitions and data](#3420) 2. [Add logarithm math utilities to kmath](#3421) 3. [Add logarithm graph state management and reducer](#3422) 4. [Add logarithm graph rendering, SR strings, and equation string](#3423) 5. [Add logarithm graph scoring](#3424) 6. [Add logarithm graph option in the Interactive Graph Editor](#3425) Add logarithm type definitions, this is the initial implementation for supporting Logarithm graph in Interactive Graph widget. - Add `PerseusGraphTypeLogarithm` and `LogarithmGraphCorrect` types to the data schema, following the exponential pattern (coords + asymptote) - Add JSON parser for the new `"logarithm"` graph type - Add `generateIGLogarithmGraph()` test data generator - Add placeholder cases in all exhaustiveness switches so the build stays green ## Details This is the first PR in the logarithm interactive graph series (LEMS-3950). It adds the type definitions with zero runtime behavior change — no graph renders, no scoring, no editor UI. **Data shape:** Logarithm follows the exponential pattern — two curve points plus an asymptote value. For logarithm the asymptote is the x-value of a vertical line (vs. exponential's y-value for a horizontal line). **Placeholder cases** were added to satisfy `UnreachableCaseError` exhaustiveness in: - `interactive-graph-editor.tsx` — graph merging switch - `start-coords/util.ts` — `shouldShowStartCoordsUI` (returns `false`), `getDefaultGraphStartCoords` (returns `undefined`) - `interactive-graph.tsx` — `getEquationString` (returns `""` — safe no-op since this is called unconditionally in the editor render path) - `initialize-graph-state.ts` — `initializeGraphState` (returns `type: "none"`) - `interactive-graph-ai-utils.ts` — `getGraphOptionsForProps` and `getUserInput` These placeholders will be replaced with real implementations in subsequent PRs. **Why `getEquationString` returns `""` instead of throwing:** `getEquationString` is called unconditionally in the editor's `render()` method (`interactive-graph-editor.tsx`). Since this PR adds the parser that accepts `type: "logarithm"`, any content with `correct.type === "logarithm"` (e.g. created via API) would reach the editor and crash the React render if the placeholder threw. All other placeholders in this PR are safe (return no-op values), but `getEquationString` is uniquely on the render path, so it must return a safe value. Future graph types should follow the same pattern: return `""` here, not throw. Issue: LEMS-3953 Co-Authored by Claude Code (Opus) ## Test plan - [ ] `pnpm tsc` passes - [ ] `pnpm knip` passes - [ ] `pnpm lint` passes - [ ] `pnpm prettier . --check` passes - [ ] Generator tests pass (2 new tests: default graph, graph with all props) - [ ] Parser regression tests pass (170 snapshots unchanged) - [ ] Interactive graph tests pass (199 tests) - [ ] Verify no runtime behavior change in Storybook (existing graph types still work) Author: ivyolamit Reviewers: claude[bot], ivyolamit, SonicScrewdriver, handeyeco Required Reviewers: Approved By: SonicScrewdriver Checks: ⏭️ 1 check has been skipped, ✅ 10 checks were successful Pull Request URL: #3420
## Summary: PR series to add logarithm graph support to the Interactive Graph widget: 1. [Add logarithm graph type definitions and data](#3420) 2.▶️ [Add logarithm math utilities to kmath](#3421) 3. [Add logarithm graph state management and reducer](#3422) 4. [Add logarithm graph rendering, SR strings, and equation string](#3423) 5. [Add logarithm graph scoring](#3424) 6. [Add logarithm graph option in the Interactive Graph Editor](#3425) Add the logarithm math utilities to kmath for supporting Logarithm graph in Interactive Graph - Add `LogarithmCoefficient` type and `getLogarithmCoefficients()` to kmath, following the exponential pattern - Compute coefficients `{a, b, c}` for `f(x) = a·ln(b·x + c)` using the inverse exponential approach - No canonical normalization needed (logarithm has no periodic equivalences, same as exponential) ## Details This adds the shared math utility for logarithm coefficient computation to `@khanacademy/kmath`, following the same pattern as `getExponentialCoefficients()`. Both the rendering component (PR 4) and scoring (PR 5) will consume this function. **Mathematical approach (inverse exponential):** 1. Flip each coordinate `(x, y) → (y, x)` — treating the logarithm as the inverse of an exponential 2. Use the asymptote x-value as the flipped exponential's `c` coefficient 4. Compute exponential coefficients `aExp`, `bExp` from the flipped points 5. Invert to get logarithm coefficients: `a = 1/bExp`, `b = 1/aExp`, `c = -cExp/aExp` **Validation guards** (returns `undefined` for invalid inputs): - Same y-coordinate on both points (makes `bExp` undefined) - A point lying on the asymptote - Points on opposite sides of the asymptote - Non-finite or zero intermediate results This matches the reference implementation in `packages/perseus-core/src/utils/grapher-util.ts` (the Grapher widget's `Logarithm` object, lines 449–558). Co-Authored by Claude Code (Opus) Issue: LEMS-3953 ## Test plan - [ ] `pnpm tsc` passes - [ ] `pnpm knip` passes - [ ] `pnpm lint` passes - [ ] `pnpm prettier . --check` passes - [ ] Coefficient tests pass (6 new tests, 17 total in coefficients.test.ts) - Grapher test data: `[-4,-3]`, `[-5,-7]`, asymptote `-6` reproduces correct y-values - Natural log: `[1,0]`, `[e,1]`, asymptote `0` → `a≈1, b≈1, c≈0` - Negative b: points left of asymptote (`y = ln(-x)`) → `b≈-1` - Same-y → `undefined` - Point on asymptote → `undefined` - Opposite sides → `undefined` Author: ivyolamit Reviewers: claude[bot], SonicScrewdriver, handeyeco Required Reviewers: Approved By: SonicScrewdriver Checks: ⏭️ 1 check has been skipped, ✅ 10 checks were successful, ⚪️ 1 check is neutral Pull Request URL: #3421
…3422) ## Summary: PR series to add logarithm graph support to the Interactive Graph widget: 1. [Add logarithm graph type definitions and data](#3420) 2. [Add logarithm math utilities to kmath](#3421) 3.▶️ [Add logarithm graph state management and reducer](#3422) 4. [Add logarithm graph rendering, SR strings, and equation string](#3423) 5. [Add logarithm graph scoring](#3424) 6. [Add logarithm graph option in the Interactive Graph Editor](#3425) Add logarithm graph state management and reducer for supporting Logarithm graph in Interactive Graph - Add `LogarithmGraphState` to the internal state type system - Wire up reducer actions (`movePoint` + `moveCenter`) with logarithm-specific constraints - Add graph state initialization with sensible defaults - Add test data fixtures and question builder support ## Details This PR adds the state management layer for logarithm graphs, following the exponential pattern throughout. Logarithm is the vertical-asymptote mirror of exponential's horizontal-asymptote design. **Action registration:** Reuses existing `movePoint` and `moveCenter` action creators (no new action types). The `actions` export object gets `logarithm: { movePoint, moveCenter }`, identical to exponential. **Reducer — `doMovePoint`:** - Point cannot land on the asymptote's x-coordinate - Both points must have different y-values (prevents degenerate coefficient computation) - Cross-asymptote reflection: when a point is dragged past the asymptote, the other point is reflected (`reflectedX = 2 * asymptoteX - otherX`) so both points end up on the same side — matches Grapher widget behavior **Reducer — `doMoveCenter`:** - Asymptote moves horizontally only (X component extracted, Y ignored) - Snap-through logic: when the new position would land between or on the curve points, snaps past all points using the midpoint heuristic for direction detection (prevents oscillation/flicker) - Final safety check: asymptote cannot land exactly on either point's x-coordinate **Initialization:** `getLogarithmCoords()` follows `getExponentialCoords()` pattern — returns `{coords, asymptote}`. Default coords use normalized fractions `[0.55, 0.55]` and `[0.75, 0.75]` to ensure both points are to the right of the default asymptote at x=0 after normalization (x=0.5 would land exactly on the asymptote). **Placeholders:** `mafs-graph.tsx` returns `{graph: null, interactiveElementsDescription: null}` for logarithm (replaced in PR 4). `mafs-state-to-interactive-graph.ts` has the real serialization (not a placeholder). Co-Authored by Claude Code (Opus) Issue: LEMS-3953 ## Test plan: - [ ] `pnpm tsc` passes - [ ] `pnpm knip` passes - [ ] `pnpm lint` passes - [ ] `pnpm prettier . --check` passes - [ ] Reducer tests pass (147 total, 14 new logarithm tests): - `movePoint`: same-y rejection, bounding-to-same-y rejection, valid move, on-asymptote rejection, cross-asymptote reflection - `moveCenter`: valid move, snap-through between points, Y-component ignored, final safety rejection - Initialization: given coords, startCoords, defaults - Gradable graph: logarithm state conversion - Serialization: logarithm state to interactive graph - [ ] Interactive graph tests pass (206 total, logarithm added to parameterized test maps) - [ ] Serialization tests pass (14 total) Author: ivyolamit Reviewers: claude[bot], ivyolamit, SonicScrewdriver Required Reviewers: Approved By: SonicScrewdriver Checks: ⏭️ 1 check has been skipped, ✅ 10 checks were successful, ⚪️ 1 check is neutral Pull Request URL: #3422
|
…uation string (#3423) ## Summary: PR series to add logarithm graph support to the Interactive Graph widget: 1. [Add logarithm graph type definitions and data](#3420) 2. [Add logarithm math utilities to kmath](#3421) 3. [Add logarithm graph state management and reducer](#3422) 4.▶️ [Add logarithm graph rendering, SR strings, and equation string](#3423) 5. [Add logarithm graph scoring](#3424) 6. [Add logarithm graph option in the Interactive Graph Editor](#3425) Create the logarithm graph visual component, add Storybook coverage, SR strings, and equation string for supporting Logarithm graph in Interactive Graph - Create the logarithm graph visual component (`logarithm.tsx`) with curve rendering, draggable asymptote, and movable points - Add 6 screen reader strings for accessibility - Add equation string display for the editor - Add Storybook story ## Details This is the largest PR in the series. It creates the visual rendering of the logarithm graph, following the exponential component pattern with the axis swapped (vertical asymptote instead of horizontal). **Curve rendering:** - Single `<Plot.OfX>` with domain restricted to one side of the asymptote (`[asymptoteX + 0.001, xMax]` or `[xMin, asymptoteX - 0.001]`) - Plot function computes `a * ln(b*x + c)`, returning NaN when outside domain or when y-value exceeds visible range + padding (prevents curve visually touching the asymptote) - `coeffRef` caches last valid coefficients as fallback during transient invalid states **Asymptote rendering:** - Uses existing `MovableAsymptote` component with `orientation="vertical"` - Dispatches `actions.logarithm.moveCenter()` on drag - `constrainAsymptoteKeyboard()` implements snap-through logic for keyboard navigation (mirrors exponential's vertical version but on X-axis) **Keyboard constraints:** - `getLogarithmKeyboardConstraint()` prevents points from landing on the asymptote's x-coordinate or sharing y-value with the other point - Uses bounded retry (max 3 steps) to skip past invalid positions **Equation string:** - `getLogarithmEquationString()` displays `y = a*ln(b*x + c)` with computed coefficient values - `defaultLogarithmCoords()` provides fallback coords (normalized fractions `[0.55, 0.55]`, `[0.75, 0.75]`) **Screen reader strings (6):** - `srLogarithmGraph` — graph container label - `srLogarithmPoint1` / `srLogarithmPoint2` — point position labels - `srLogarithmAsymptote` — asymptote label with keyboard instructions - `srLogarithmDescription` — graph state description - `srLogarithmInteractiveElements` — interactive elements summary Co-Authored by Claude Code (Opus) Issue: LEMS-3953 ## Test plan: - [ ] `pnpm tsc` passes - [ ] `pnpm knip` passes - [ ] `pnpm lint` passes - [ ] `pnpm prettier . --check` passes - [ ] Logarithm component tests pass (15 new tests): - 9 screen reader tests (aria-labels, descriptions, interactive elements, updates on state change) - 3 keyboard constraint tests for points (valid move, skip asymptote, skip same-y) - 3 keyboard constraint tests for asymptote (free move, snap-through, skip point x-value) - [ ] Interactive graph tests pass (178 passed, 28 skipped) - [ ] Verify rendering in Storybook (logarithm story renders correctly, curve matches expected shape) Author: ivyolamit Reviewers: claude[bot], ivyolamit, SonicScrewdriver Required Reviewers: Approved By: SonicScrewdriver Checks: ⏭️ 1 check has been skipped, ✅ 10 checks were successful Pull Request URL: #3423
SonicScrewdriver
left a comment
There was a problem hiding this comment.
I APPROVE AGAIN!
680fbf2 to
886aa19
Compare
## Summary: PR series to add logarithm graph support to the Interactive Graph widget: 1. [Add logarithm graph type definitions and data](#3420) 2. [Add logarithm math utilities to kmath](#3421) 3. [Add logarithm graph state management and reducer](#3422) 4. [Add logarithm graph rendering, SR strings, and equation string](#3423) 5.▶️ [Add logarithm graph scoring](#3424) 6. [Add logarithm graph option in the Interactive Graph Editor](#3425) Add logarithm graph scoring to support the Logarithm graph in Interactive Graph - Add logarithm scoring to `score-interactive-graph.ts` using direct coefficient comparison - No canonical normalization needed (same as exponential) ## Details Follows the exponential scoring pattern exactly. Computes `{a, b, c}` coefficients for both the user's answer and the rubric using `getLogarithmCoefficients` from kmath, then compares with `approximateDeepEqual`. **Scoring logic:** - Returns `invalid` if guess coords or asymptote are missing, or if coefficient computation fails for either side - Returns `earned: 1` if `[a, b, c]` coefficients approximately match - Returns `earned: 0` otherwise This means two different sets of control points that define the same logarithmic curve will score as correct — the comparison is on the mathematical function, not the specific points chosen. Co-Authored by Claude Code (Opus) Issue: LEMS-3953 ## Test plan: - [ ] `pnpm tsc` passes - [ ] `pnpm knip` passes - [ ] `pnpm lint` passes - [ ] `pnpm prettier . --check` passes - [ ] Scoring tests pass (44 total, 6 new logarithm tests): - [ ] Invalid: undefined guess, null coords, null asymptote - [ ] Correct: matching coefficients - [ ] Incorrect: different coefficients - [ ] Equivalent curves: different control points producing same `y = ln(x)` coefficients → correct Author: ivyolamit Reviewers: claude[bot], SonicScrewdriver Required Reviewers: Approved By: SonicScrewdriver Checks: ⏭️ 1 check has been skipped, ✅ 10 checks were successful Pull Request URL: #3424
…: Add logarithm graph option in the Interactive Graph Editor
…aph] Add logarithm graph option in the Interactive Graph Editor
53155ac to
5bc53da
Compare
…related to wonderblocks
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @khanacademy/kmath@2.4.0 ### Minor Changes - [#3421](#3421) [`9099a40a4e`](9099a40) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add the logarithm math utilities to kmath for supporting Logarithm graph in Interactive Graph ### Patch Changes - Updated dependencies \[[`a72f7db4a3`](a72f7db)]: - @khanacademy/perseus-core@24.1.0 ## @khanacademy/perseus@77.2.0 ### Minor Changes - [#3422](#3422) [`a4eaa5a1f8`](a4eaa5a) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add logarithm graph state management and reducer for supporting Logarithm graph in Interactive Graph - [#3425](#3425) [`2c36b8a4c3`](2c36b8a) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add logarithm graph option in the Interactive Graph Editor - [#3423](#3423) [`3cc56f60dd`](3cc56f6) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Create the logarithm graph visual component, add Storybook coverage, SR strings, and equation string for supporting Logarithm graph in Interactive Graph - [#3420](#3420) [`a72f7db4a3`](a72f7db) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add logarithm type definitions, this is the initial implementation for supporting Logarithm graph in Interactive Graph widget. ### Patch Changes - [#3461](#3461) [`0481b4238d`](0481b42) Thanks [@nishasy](https://github.com/nishasy)! - [Image] | (DX) | Update CSS modules to kabab case - [#3455](#3455) [`0d6cf3c3ce`](0d6cf3c) Thanks [@nishasy](https://github.com/nishasy)! - [Image] | (UX) | Fix messed up Graphie labels behind scale feature flag - Updated dependencies \[[`08b1218b62`](08b1218), [`a72f7db4a3`](a72f7db), [`9099a40a4e`](9099a40)]: - @khanacademy/perseus-score@8.6.0 - @khanacademy/perseus-core@24.1.0 - @khanacademy/kmath@2.4.0 - @khanacademy/keypad-context@3.2.42 - @khanacademy/math-input@26.4.13 - @khanacademy/perseus-linter@4.9.2 ## @khanacademy/perseus-core@24.1.0 ### Minor Changes - [#3420](#3420) [`a72f7db4a3`](a72f7db) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add logarithm type definitions, this is the initial implementation for supporting Logarithm graph in Interactive Graph widget. ## @khanacademy/perseus-editor@30.2.0 ### Minor Changes - [#3425](#3425) [`2c36b8a4c3`](2c36b8a) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add logarithm graph option in the Interactive Graph Editor - [#3420](#3420) [`a72f7db4a3`](a72f7db) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add logarithm type definitions, this is the initial implementation for supporting Logarithm graph in Interactive Graph widget. ### Patch Changes - [#3461](#3461) [`0481b4238d`](0481b42) Thanks [@nishasy](https://github.com/nishasy)! - [Image] | (DX) | Update CSS modules to kabab case - Updated dependencies \[[`a4eaa5a1f8`](a4eaa5a), [`2c36b8a4c3`](2c36b8a), [`3cc56f60dd`](3cc56f6), [`08b1218b62`](08b1218), [`a72f7db4a3`](a72f7db), [`9099a40a4e`](9099a40), [`0481b4238d`](0481b42), [`0d6cf3c3ce`](0d6cf3c)]: - @khanacademy/perseus@77.2.0 - @khanacademy/perseus-score@8.6.0 - @khanacademy/perseus-core@24.1.0 - @khanacademy/kmath@2.4.0 - @khanacademy/keypad-context@3.2.42 - @khanacademy/math-input@26.4.13 - @khanacademy/perseus-linter@4.9.2 ## @khanacademy/perseus-score@8.6.0 ### Minor Changes - [#3424](#3424) [`08b1218b62`](08b1218) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add logarithm graph scoring to support the Logarithm graph in Interactive Graph ### Patch Changes - Updated dependencies \[[`a72f7db4a3`](a72f7db), [`9099a40a4e`](9099a40)]: - @khanacademy/perseus-core@24.1.0 - @khanacademy/kmath@2.4.0 ## @khanacademy/keypad-context@3.2.42 ### Patch Changes - Updated dependencies \[[`a72f7db4a3`](a72f7db)]: - @khanacademy/perseus-core@24.1.0 ## @khanacademy/math-input@26.4.13 ### Patch Changes - Updated dependencies \[[`a72f7db4a3`](a72f7db)]: - @khanacademy/perseus-core@24.1.0 - @khanacademy/keypad-context@3.2.42 ## @khanacademy/perseus-linter@4.9.2 ### Patch Changes - Updated dependencies \[[`a72f7db4a3`](a72f7db), [`9099a40a4e`](9099a40)]: - @khanacademy/perseus-core@24.1.0 - @khanacademy/kmath@2.4.0

Summary:
PR series to add logarithm graph support to the Interactive Graph widget:
Add logarithm graph option in the Interactive Graph Editor
StartCoordsLogarithmcomponent for configuring logarithm start coordinates in the editorinteractive-graph-logarithmfeature flaggetLogarithmCoordsfor use by the editorDetails
This PR enables content creators to configure logarithm exercises in the Interactive Graph editor, following the exponential editor pattern.
StartCoordsLogarithm component:
y = a * ln(b*x + c)with computed coefficient valuesstart-coords-exponential.module.cssFeature flag: "Logarithm function" appears in the graph type selector only when
interactive-graph-logarithmis enabled, preventing content creators from selecting it until it's ready for launch.Editor validation: The start asymptote x-value is validated — it cannot fall between or on the x-coordinates of the curve's start points (mirrors exponential's y-axis validation but for x-axis).
Exports:
getLogarithmCoords()is now exported frominitialize-graph-state.tsand re-exported from@khanacademy/perseusfor use by the editor's start-coords UI.Co-Authored by Claude Code (Opus)
Issue: LEMS-3969
Test plan:
pnpm tscpassespnpm knippassespnpm lintpassespnpm prettier . --checkpasses